Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: lazy load of which module #176

Merged
merged 2 commits into from
Dec 19, 2023
Merged

Conversation

thecodrr
Copy link
Contributor

Currently @npmcli/git is eagerly getting and caching the git path in a user's system which can have a significant performance cost when requiring @npmcli/git. This PR avoids this cost by lazily getting & caching git path on first use.

References

@thecodrr thecodrr requested a review from a team as a code owner December 17, 2023 16:59
@wraithgar
Copy link
Member

Can't the same benefit be reached by moving the require('./which.js') in lib/spawn.js to be inside the exported function? Then the results can be cached even in the event there is no git present.

@thecodrr
Copy link
Contributor Author

Can't the same benefit be reached by moving the require('./which.js') in lib/spawn.js to be inside the exported function? Then the results can be cached even in the event there is no git present.

Got it.

@thecodrr
Copy link
Contributor Author

@wraithgar should be all good.

@wraithgar wraithgar changed the title Lazily get and cache git path on first use fix: lazy load of which module Dec 19, 2023
@wraithgar wraithgar merged commit c398872 into npm:main Dec 19, 2023
20 of 21 checks passed
@github-actions github-actions bot mentioned this pull request Dec 19, 2023
@thecodrr thecodrr deleted the defer-finding-git branch December 19, 2023 02:38
wraithgar pushed a commit that referenced this pull request Jan 4, 2024
🤖 I have created a release *beep* *boop*
---


## [5.0.4](v5.0.3...v5.0.4)
(2023-12-19)

### Bug Fixes

*
[`c398872`](c398872)
[#176](#176) lazy load of which module
(#176) (@thecodrr)

### Chores

*
[`46192f5`](46192f5)
[#175](#175) postinstall for dependabot
template-oss PR (@lukekarrys)
*
[`51011f8`](51011f8)
[#175](#175) bump @npmcli/template-oss
from 4.21.1 to 4.21.3 (@dependabot[bot])
*
[`8b1d897`](8b1d897)
[#173](#173) postinstall for dependabot
template-oss PR (@lukekarrys)
*
[`157ed48`](157ed48)
[#173](#173) bump @npmcli/template-oss
from 4.19.0 to 4.21.1 (@dependabot[bot])
*
[`0d06a9f`](0d06a9f)
[#154](#154) postinstall for dependabot
template-oss PR (@lukekarrys)
*
[`b425d28`](b425d28)
[#154](#154) bump @npmcli/template-oss
from 4.18.1 to 4.19.0 (@dependabot[bot])
*
[`ecc938a`](ecc938a)
[#152](#152) postinstall for dependabot
template-oss PR (@lukekarrys)
*
[`72430ad`](72430ad)
[#152](#152) bump @npmcli/template-oss
from 4.18.0 to 4.18.1 (@dependabot[bot])

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants